Automated Test: feature-rolling-updates-implementation #319
Conversation
Closes #36840 Signed-off-by: Pedro Ruivo <pruivo@redhat.com>
📝 WalkthroughWalkthroughThe PR introduces a new "Rolling Updates" preview feature to Keycloak. It adds a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
operator/src/test/java/org/keycloak/operator/testsuite/integration/UpgradeTest.java (1)
102-116:⚠️ Potential issue | 🟠 MajorEnable rolling-updates feature for all strategy paths, including when updateStrategy is null.
The test at line 94-95 expects rolling upgrade behavior when
updateStrategy == null, but the ROLLING_UPDATES feature is only enabled for non-null strategies (lines 112-115). Move the feature setup before the early return to ensure it applies to all paths.Suggested adjustment
private static Keycloak createInitialDeployment(UpdateStrategy updateStrategy) { var kc = getTestKeycloakDeployment(true); kc.getSpec().setInstances(3); + if (kc.getSpec().getFeatureSpec() == null) { + kc.getSpec().setFeatureSpec(new FeatureSpec()); + } + kc.getSpec().getFeatureSpec().setEnabledFeatures(List.of(Profile.Feature.ROLLING_UPDATES.getKey())); + if (updateStrategy == null) { return kc; } var updateSpec = new UpdateSpec(); updateSpec.setStrategy(updateStrategy); kc.getSpec().setUpdateSpec(updateSpec); - - if (kc.getSpec().getFeatureSpec() == null) { - kc.getSpec().setFeatureSpec(new FeatureSpec()); - } - kc.getSpec().getFeatureSpec().setEnabledFeatures(List.of(Profile.Feature.ROLLING_UPDATES.getKey())); return kc; }
🤖 Fix all issues with AI agents
In `@docs/guides/operator/advanced-configuration.adoc`:
- Around line 446-450: Reword the CAUTION block to fix grammar and clarify the
actual failure mode: state that while the rolling-updates feature is in preview,
attempting to run the update compatibility check commands (kc update check and
kc update metadata) will exit with a feature-disabled status; the {project_name}
Operator itself continues to operate normally. Replace "While on preview stage"
with "While the feature is in preview" (or "While in preview stage") and make
clear this limitation applies only to using the rolling-updates compatibility
checks, not general Operator functionality.
In `@docs/guides/server/update-compatibility.adoc`:
- Around line 132-134: Update the documentation text that currently maps exit
code 4 to "Rolling Upgrade is not possible" so it accurately reflects that the
command aborted because the preview feature is disabled; change the wording for
exit code 4 to something like "Command cannot run because the preview feature
`rolling-updates` is disabled" and ensure the doc line referencing exit code 4
(the entry showing "Rolling Upgrade is not possible" and mention of
`rolling-updates`) is replaced accordingly.
In
`@quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/compatibility/CompatibilityResult.java`:
- Around line 31-35: The change swapped semantics of exit codes so external
tooling expecting recreate-upgrade on code 4 will break; restore backward
compatibility by keeping RECREATE_UPGRADE_EXIT_CODE = 4 (or add a
LEGACY_RECREATE_UPGRADE_EXIT_CODE = 4) and assign FEATURE_DISABLED to a
different code (e.g., 5), or introduce both constants
(RECREATE_UPGRADE_EXIT_CODE = 4 and FEATURE_DISABLED = 5) and mark the old
mapping as deprecated; update the CompatibilityResult constants
(RECREATE_UPGRADE_EXIT_CODE, FEATURE_DISABLED or add
LEGACY_RECREATE_UPGRADE_EXIT_CODE) and add a short deprecation comment so
callers/tools can adapt.
| [CAUTION] | ||
| ==== | ||
| While on preview stage, the feature `rolling-updates` must be enabled. | ||
| Otherwise, the {project_name} Operator will fail. | ||
| ==== |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the file context around lines 446-450
head -n 480 docs/guides/operator/advanced-configuration.adoc | tail -n 50Repository: admin-coderabbit/coderabbit-keycloak-mirror
Length of output: 1973
🏁 Script executed:
# Search for rolling-updates feature mentions in the codebase
rg "rolling-updates" --type adoc --type md --type java --type yaml -B 2 -A 2Repository: admin-coderabbit/coderabbit-keycloak-mirror
Length of output: 115
🏁 Script executed:
# Look for operator failure behavior related to rolling-updates
rg "rolling.?update" -i --type java -B 3 -A 3 | head -100Repository: admin-coderabbit/coderabbit-keycloak-mirror
Length of output: 12841
🏁 Script executed:
# Check if there's operator code that validates rolling-updates enablement
rg "rolling" --type java -l | head -20Repository: admin-coderabbit/coderabbit-keycloak-mirror
Length of output: 1464
🏁 Script executed:
# Search for validation of rolling-updates feature
rg "rolling.?update" -i --type java -B 5 -A 5 | grep -E "(fail|error|throw|require|validate)" -B 3 -A 3Repository: admin-coderabbit/coderabbit-keycloak-mirror
Length of output: 5057
🏁 Script executed:
# Search for feature validation logic
rg "features.*enabled" --type java -B 3 -A 3 | head -80Repository: admin-coderabbit/coderabbit-keycloak-mirror
Length of output: 9971
🏁 Script executed:
# Look for upgrade logic that might require rolling-updates
rg "UpgradeType|ROLLING|RECREATE" --type java | grep -i "strategy\|feature" | head -20Repository: admin-coderabbit/coderabbit-keycloak-mirror
Length of output: 1150
🏁 Script executed:
# Check BaseUpgradeLogic which likely has upgrade handling
cat -n operator/src/main/java/org/keycloak/operator/upgrade/impl/BaseUpgradeLogic.java | head -150Repository: admin-coderabbit/coderabbit-keycloak-mirror
Length of output: 7218
🏁 Script executed:
# Find and read UpdateCompatibilityCheck.java
find . -name "UpdateCompatibilityCheck.java" -type f -exec cat -n {} \;Repository: admin-coderabbit/coderabbit-keycloak-mirror
Length of output: 3976
🏁 Script executed:
# Find and read UpdateCompatibilityMetadata.java
find . -name "UpdateCompatibilityMetadata.java" -type f -exec cat -n {} \;Repository: admin-coderabbit/coderabbit-keycloak-mirror
Length of output: 4631
🏁 Script executed:
# Search for any error or failure behavior when rolling-updates feature is absent
rg "ROLLING_UPDATES" --type java -B 5 -A 5Repository: admin-coderabbit/coderabbit-keycloak-mirror
Length of output: 5394
Clarify the specific failure mode and fix grammar.
The CAUTION states the Operator "will fail" if rolling-updates is not enabled, but the actual behavior is more limited: the update compatibility check commands (kc update check and kc update metadata) exit with a feature-disabled status when the feature is disabled. The Operator itself continues to function; it does not prevent general operation. Consider: "While the feature is in preview stage" (or "While in preview stage") instead of "While on preview stage" for correct grammar. Reword to clarify that this constraint applies to using rolling update compatibility checks, not to the Operator as a whole.
🤖 Prompt for AI Agents
In `@docs/guides/operator/advanced-configuration.adoc` around lines 446 - 450,
Reword the CAUTION block to fix grammar and clarify the actual failure mode:
state that while the rolling-updates feature is in preview, attempting to run
the update compatibility check commands (kc update check and kc update metadata)
will exit with a feature-disabled status; the {project_name} Operator itself
continues to operate normally. Replace "While on preview stage" with "While the
feature is in preview" (or "While in preview stage") and make clear this
limitation applies only to using the rolling-updates compatibility checks, not
general Operator functionality.
| m|4 | ||
| |Rolling Upgrade is not possible. | ||
| The feature `rolling-updates` is disabled. |
There was a problem hiding this comment.
Reword exit-code 4 to reflect “feature disabled.”
Exit code 4 is returned before any compatibility evaluation, so describing it as “Rolling Upgrade is not possible” is misleading. Suggest wording like “Command cannot run because the preview feature rolling-updates is disabled.”
✏️ Suggested doc tweak
-m|4
-|Rolling Upgrade is not possible.
-The feature `rolling-updates` is disabled.
+m|4
+|Command cannot run because the preview feature `rolling-updates` is disabled.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| m|4 | |
| |Rolling Upgrade is not possible. | |
| The feature `rolling-updates` is disabled. | |
| m|4 | |
| |Command cannot run because the preview feature `rolling-updates` is disabled. |
🤖 Prompt for AI Agents
In `@docs/guides/server/update-compatibility.adoc` around lines 132 - 134, Update
the documentation text that currently maps exit code 4 to "Rolling Upgrade is
not possible" so it accurately reflects that the command aborted because the
preview feature is disabled; change the wording for exit code 4 to something
like "Command cannot run because the preview feature `rolling-updates` is
disabled" and ensure the doc line referencing exit code 4 (the entry showing
"Rolling Upgrade is not possible" and mention of `rolling-updates`) is replaced
accordingly.
| // see picocli.CommandLine.ExitCode | ||
| // 1 -> software error | ||
| // 2 -> usage error | ||
| int RECREATE_UPGRADE_EXIT_CODE = 3; | ||
| int FEATURE_DISABLED = 4; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for references to RECREATE_UPGRADE_EXIT_CODE and exit code values
rg -n "RECREATE_UPGRADE_EXIT_CODE|FEATURE_DISABLED" --type java
# Check for test files that might reference exit codes
rg -n "exitCode|exit.*4|exit.*3" --type java -A 2 -B 2 | head -100
# Search for documentation files mentioning exit codes
fd -e adoc -e md | xargs grep -l "exit.code\|exitCode" 2>/dev/null || trueRepository: admin-coderabbit/coderabbit-keycloak-mirror
Length of output: 15287
🏁 Script executed:
# More focused search: find where these constants are used
rg -n "return RECREATE_UPGRADE_EXIT_CODE|return.*4|System.exit" --type java -A 2 -B 2
# Check the IncompatibilityResult class mentioned in original scratchpad
fd -n "IncompatibilityResult.java" --type fRepository: admin-coderabbit/coderabbit-keycloak-mirror
Length of output: 50648
Breaking change: Exit code 4 now has different semantics.
Previously, exit code 4 indicated that a recreate upgrade was required. After this change, exit code 4 now indicates that a feature is disabled, while exit code 3 indicates recreate upgrade is required. Any external scripts or automation tools checking for exit code 4 to detect the need for a recreate upgrade scenario will break and require updates.
🤖 Prompt for AI Agents
In
`@quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/compatibility/CompatibilityResult.java`
around lines 31 - 35, The change swapped semantics of exit codes so external
tooling expecting recreate-upgrade on code 4 will break; restore backward
compatibility by keeping RECREATE_UPGRADE_EXIT_CODE = 4 (or add a
LEGACY_RECREATE_UPGRADE_EXIT_CODE = 4) and assign FEATURE_DISABLED to a
different code (e.g., 5), or introduce both constants
(RECREATE_UPGRADE_EXIT_CODE = 4 and FEATURE_DISABLED = 5) and mark the old
mapping as deprecated; update the CompatibilityResult constants
(RECREATE_UPGRADE_EXIT_CODE, FEATURE_DISABLED or add
LEGACY_RECREATE_UPGRADE_EXIT_CODE) and add a short deprecation comment so
callers/tools can adapt.
This pull request was automatically created by
@coderabbitai/e2e-reviewer.Batch created pull request.
Summary by CodeRabbit
Release Notes
New Features
Documentation